Repository navigation
fix(types): Accept attribute paths in plotly_relayout, relayout, restyle, and update - #8126
camdecoster wants to merge 3 commits into
Conversation
| - name: Type-check TypeScript sources | ||
| run: npm run typecheck | ||
|
|
||
| test-types: |
There was a problem hiding this comment.
nit: could we just add test-types as a second step in the typecheck job above? Is there a reason you want a second runner?
| /** | ||
| * Consumer-side checks for the `relayout`, `restyle` and `update` types. | ||
| * | ||
| * Nothing here runs. `npm run typecheck` compiles this file against the public entry point. |
There was a problem hiding this comment.
nit:
| * Nothing here runs. `npm run typecheck` compiles this file against the public entry point. | |
| * Nothing here runs. `npm run test-types` compiles this file against the public entry point. |
| * autorange keys that flatten nested updates. | ||
| */ | ||
| export interface PlotRelayoutEvent extends Partial<Layout> { | ||
| export interface PlotRelayoutEvent extends LayoutUpdate { |
There was a problem hiding this comment.
This type contains 6 hand-crafted keys (xaxis.range[0], etc.) but LayoutUpdate doesn't type these keys at all.
This starts to look like 2 sources of truth for these types. And already, the two definitions are drifting:
the hand-crafted keys here define values that are narrower than in LayoutUpdate. xaxis.range[0]?: number misses date axes with values like 2026-10-16.
What do you think about moving the hand-typed keys here into LayoutUpdate with the type widened to include date strings? That way there would be a single definition.
There was a problem hiding this comment.
The overloads you add here are good, but can still report "errors" on perfectly valid code.
Example:
import * as Plotly from 'plotly.js';
type RelayoutArg = Parameters<typeof Plotly.relayout>[1];
const wrapped: RelayoutArg = { autosize: true };There is a red squiggle as typescript evaluates against the "last" overload and completely misses the other valid overload.
Typescript says to put the most generic signature last. What do you think about including a generic "catch-all" overload at the end for these kinds of cases?
export function relayout(root: Root, astr: string | LayoutUpdate, val?: any): Promise<PlotlyHTMLElement>;The same comment applies to the restyle overloads.
2 participants
Description
Update types to accept attribute paths in
plotly_relayout,relayout,restyle, andupdate.Closes #8047.
Changes
Testing
test-typesNotes
anyis a compromise so that we don't have to generate a list of any possible path string